Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Potential fix for Delicious addLink error #55

Merged
merged 3 commits into from
May 25, 2014

Conversation

mmerriam
Copy link
Contributor

Sorry it took so long, but I just started a new job this week, and my sublime JS beautifier plugin made an initial mess of the original code.
This may not fix everyone's recent problems, but after stepping through the code, I noticed that all requests to the Delicious API with non-ascii chars (in the description field, from the HTML title) were getting rejected.
All I did was put a regex filter on it. So far, I haven't had any difficulty saving links anymore.
Also, my git skills are lacking and I couldn't figure out how to squash the earlier commit (which included all the JS beautifier adjustments), so I just reverted the initial change (5/21).
Thanks.

mmerriam added 3 commits May 21, 2014 23:29
…is is bc the API is failing on addLink calls when crazy chars are found in the description field (pulled from title value). domainersuitedev/delicious-api#22
…hars. This is bc the API is failing on addLink calls when crazy chars are found in the description field (pulled from title value). domainersuitedev/delicious-api#22"

This reverts commit 8677690.
filter any non-ascii chars from the title value, because Delicious does
not handle non-ascii chars well.  Returns error on POST to addLink
@zmanring-zz
Copy link
Owner

This looks good man, can you provide me with something I can test with to make sure it didnt break anything else? Maybe a page that has ascii characters? Thanks again for your help, I need to also make sure its pushed into version 3, but I can handle that manually.

@mmerriam
Copy link
Contributor Author

Glad to help out.
Are you asking for automated tests, or just URLs to manually test pages?
I've been manually testing the change on my machine (lots of links to save :)). So far, all POSTs and PUTs have been successful.

Last known 'problem' URL (now saving): http://me.dt.in.th/page/JavaScript-override/
DeliciousAPI 'issues' page: domainersuitedev/delicious-api#22

I can make some time in the next few weeks to help beta test v3, if you're interested.

zmanring-zz pushed a commit that referenced this pull request May 25, 2014
Potential fix for Delicious addLink error
@zmanring-zz zmanring-zz merged commit e995264 into zmanring-zz:develop May 25, 2014
@zmanring-zz
Copy link
Owner

Ok, so this does work, but I don't see it as a long term fix. If you go to Delicious.com and add the links they work fine, there is something missing when sending it to the API. I am digging into it now.

Cheers.

@mmerriam
Copy link
Contributor Author

Agreed.
But if you notice, the Delicious site seems to be detecting non-ascii's on the title, and using the URL domain instead (for the title/decription field).
Compare "problem" site:
http://me.dt.in.th/page/JavaScript-override/
With "normal" site:
http://meyerweb.com/eric/tools/dencoder/

As another test, when I double-encode the value from the extension's POST call, then Delicious will save the value properly, and even display the 'star' character on their site. However, then the extension is double-encoding the value, and the 'my links' view displays the encoded value. Ugh.

Outside of fixing the temporary filter, it looks like we may need to add non-ascii detection and either:

  • double-encode the char(s) that 'fail'
    or
  • substitute the URL domain, like Delicious does now

Thanks for hearing me out. Least I can do is buy you a beer this summer.

@zmanring-zz
Copy link
Owner

@mmerriam, I just pushed up my changes. I through it into a function that can be expanded upon later. Using this page as an example.
http://www.kjell.com/sortiment/el/el-produkter/starkstrom/kablar/apparatkablar/forlangning-c13-14-2-0-m-p67001

I didn't that it was removing the ö and ä. I found the translation of that on stack overflow.
http://stackoverflow.com/questions/1271567/how-do-i-replace-accents-german-in-net

Go ahead and pull down the latest from develop 663cfc4 and let me know if it works in your testing. If so ill push to Chrome Webstore.

@zmanring-zz
Copy link
Owner

I just thought of something, this will break it for other languages.

https://ja.wikipedia.org/wiki/%E4%B8%96%E7%95%8C%E9%81%BA%E7%94%A3

If you try to save that it removes all the Japanese characters, thats not cool.

@mmerriam
Copy link
Contributor Author

I agree.
See new pull request.
Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants